Allow reviewed exceptions to related-hold blocks by jazairi · Pull Request #1545 · MITLibraries/thing

jazairi · 2026-11-09T23:08:20Z

Why these changes are being introduced:

An author may legitimately have multiple distinct
theses with holds. We need an affordance for
processors to override the publication block in
such cases.

Relevant ticket(s):

How this addresses that need:

Adds a field to the the processing form that allows processors to override the publication
block for related theses.
Manages exceptions to publication blocks in a model that's backed by PaperTrail.
Refreshes publication readiness when holds are saved, reassigned, or deleted.
Rechecks holds before queuing publication and when publication jobs run.

Side effects of this change:

Existing stale readiness statuses are not
automatically fixed. However, this is a highly
unlikely edge case.

Developer

Accessibility

ANDI or WAVE has been run in accordance to our guide.
This PR contains no changes to the view layer.
New issues flagged by ANDI or WAVE have been resolved.
New issues flagged by ANDI or WAVE have been ticketed (link in the Pull Request details above).
No new accessibility issues have been flagged.

New ENV

All new ENV is documented in README.
All new ENV has been added to Heroku Pipeline, Staging and Prod.
ENV has not changed.

Approval beyond code review

UXWS/stakeholder approval has been confirmed.
UXWS/stakeholder review will be completed retroactively.
UXWS/stakeholder review is not needed.

Additional context needed to review

It's easiest to test using the preseeded data in the staging db. I've loaded a copy of that db into the review app.

Code Reviewer

Code

I have confirmed that the code works as intended.
Any CodeClimate issues have been fixed or confirmed as
added technical debt.

Documentation

The commit message is clear and follows our guidelines
(not just this pull request message).
The documentation has been updated or is unnecessary.
New dependencies are appropriate or there were no changes.

Testing

There are appropriate tests covering any new functionality.
No additional test coverage is required.

Why these changes are being introduced: An author may legitimately have multiple distinct theses with holds. We need an affordance for processors to override the publication block in such cases. Relevant ticket(s): - [ETD-704](https://mitlibraries.atlassian.net/browse/ETD-704) - [ETD-689](https://mitlibraries.atlassian.net/browse/ETD-689) How this addresses that need: - Adds a field to the the processing form that allows processors to override the publication block for related theses. - Manages exceptions to publication blocks in a model that's backed by PaperTrail. - Refreshes publication readiness when holds are saved, reassigned, or deleted. - Rechecks holds before queuing publication and when publication jobs run. Side effects of this change: Existing stale readiness statuses are not automatically fixed. However, this is a highly unlikely edge case.

coveralls · 2026-11-09T23:12:06Z

coverage: 98.382% (+0.08%) from 98.298% — etd-704 into main

Copilot

🟡 Changes recommended

The change introduces synchronous N+1 queries across publication batches and can reject unchanged unchecked exceptions after hold removal, discarding metadata edits.

2 open issues

What changed in this PR

This PR lets processors waive publication blocks caused by holds on distinct related theses, while preserving direct holds and other readiness requirements.

Changes:

Adds directional, audited exceptions and controls to grant or revoke them in the processing form.
Refreshes readiness when holds change and rechecks holds before queuing and executing publication.
Adds regression tests for authorization, audit history, rollback, and publication safeguards.

File	Description
test/models/related_hold_exception_test.rb	Tests exception validation, persistence, rollback, and cleanup.
test/models/hold_test.rb	Tests readiness updates after hold changes.
test/jobs/dspace_publication_job_test.rb	Tests publication safeguards against current holds.
test/controllers/thesis_controller_test.rb	Tests review permissions, processing updates, and publication filtering.
db/schema.rb	Records the exception table and constraints.
db/migrate/20261105120000_create_related_hold_exceptions.rb	Creates directional exceptions with database integrity checks.
app/views/thesis/process_thesis.html.erb	Adds related-thesis details and exception checkboxes.
app/models/thesis.rb	Applies exceptions to readiness and saves reviews transactionally.
app/models/related_hold_exception.rb	Validates and audits exception grants and revocations.
app/models/hold.rb	Refreshes affected theses after hold saves and deletions.
app/jobs/dspace_publication_job.rb	Rechecks holds before publication proceeds.
app/controllers/thesis_controller.rb	Handles review submissions and queues eligible publication candidates.

_{Review effort: Balanced}

+    filter_theses_by_term(Thesis.in_review).select do |thesis|
+      thesis.no_active_holds? && thesis.no_other_theses_with_holds?
+    end


Why these changes are being introduced: A copilot review noted that stale unchecked hold reviews could reject valid metadata edits. Additionally, publication candidate selection led n+1 queries. Relevant ticket(s): - [ETD-704](https://mitlibraries.atlassian.net/browse/ETD-704) How this addresses that need: - Absent unchecked related-thesis exceptions are now treated as no-ops. - Publication candidates are now selected through one SQL query. Side effects of this change: We don't typically insert SQL queries in scopes, but it seemed like the least-bad option in this case.

Copilot

🔵 Needs a closer look

The attachment-race retry can reuse a stale association from the rolled-back attempt, skip recreating the exception, and lose the processor’s decision; reload the association before building the lookup.

1 open issue

Batch hold eligibility checks to avoid N+1 queries

1 previously missed issue

In code that hasn't changed since last review

Reload persisted exceptions before retrying attachment association

app/models/related_hold_exception.rb:48

On the attachment-race retry, this association can still contain the exception created during the first attempt even though that attempt's transaction rolled back. Reusing the same thesis object then treats the rolled-back exception as existing, so line 52 skips recreating it and the successful retry loses the processor's decision. Reload the association before building this lookup so each attempt uses persisted exceptions.

1 resolved since last review

Treat stale unchecked exceptions as idempotent no-ops

_{Review effort: Balanced}

mitlib temporarily deployed to thesis-submit-pr-1545 November 9, 2026 23:08 Inactive

jazairi requested a balanced review from Copilot November 9, 2026 23:31

Copilot started reviewing on behalf of jazairi November 9, 2026 23:31

View session

Copilot AI reviewed Nov 9, 2026

View reviewed changes

Comment thread app/controllers/thesis_controller.rb Outdated

Comment on lines +272 to +274

filter_theses_by_term(Thesis.in_review).select do |thesis|

thesis.no_active_holds? && thesis.no_other_theses_with_holds?

end

Comment thread app/models/related_hold_exception.rb Resolved

jazairi requested a balanced review from Copilot November 13, 2026 00:40

jazairi deployed to thesis-submit-pr-1545 November 13, 2026 00:40

View deployment

Copilot started reviewing on behalf of jazairi November 13, 2026 00:40

View session

Copilot AI reviewed Nov 13, 2026

View reviewed changes

Navigation Menu

Repository navigation

Allow reviewed exceptions to related-hold blocks - #1545

Allow reviewed exceptions to related-hold blocks#1545
jazairi wants to merge 2 commits into
mainfrom
etd-704

jazairi commented Nov 9, 2026 •

edited by atlassian Bot

Loading

Uh oh!

coveralls commented Nov 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jazairi commented Nov 9, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why these changes are being introduced:

Relevant ticket(s):

How this addresses that need:

Side effects of this change:

Developer

Accessibility

New ENV

Approval beyond code review

Additional context needed to review

Code Reviewer

Code

Documentation

Testing

Uh oh!

coveralls commented Nov 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jazairi commented Nov 9, 2026 •

edited by atlassian Bot

Loading

coveralls commented Nov 9, 2026 •

edited

Loading